Skip to content

feat: merge-train/barretenberg#22324

Merged
iakovenkos merged 9 commits intonextfrom
merge-train/barretenberg
Apr 6, 2026
Merged

feat: merge-train/barretenberg#22324
iakovenkos merged 9 commits intonextfrom
merge-train/barretenberg

Conversation

@AztecBot
Copy link
Copy Markdown
Collaborator

@AztecBot AztecBot commented Apr 6, 2026

BEGIN_COMMIT_OVERRIDE
fix: reject VK with log_circuit_size=0 in UltraKeccak verifier (#22319)
chore: merkle tree audit 2 (#21475)
fix: graceful failures in verifier code paths + other fixes (#22311)
fix: Fr::from_u64 big-endian encoding to match C++ msgpack format (#22233)
fix: corrupt low-order bytes in batch verifier test to avoid non-canonical field encoding (#22333)
fix: skip MsgpackRejectsNonCanonical test in WASM builds (#22335)
END_COMMIT_OVERRIDE

## Summary

Fixes a memory-safety vulnerability in the native UltraKeccak non-ZK
verifier where a malformed verification key with `log_circuit_size = 0`
causes an out-of-bounds write in `get_dyadic_powers_of_challenge`.

---------

Co-authored-by: ludamad <adam.domurad@gmail.com>
AztecBot and others added 4 commits April 6, 2026 12:50
- Addresses TODO #17755 by resolving the bounds-related issues and
documenting the current leaf-read semantics in
`content_addressed_append_only_tree.hpp`:
  - `leaf_index >= max_size_` → failure (out of tree range)
- `leaf_index < max_size_` but leaf not written → return 0 with `success
= true`
- (Historical reads at a block) `leaf_index >= blockData.size` → failure
(out of block range)
- (Historical reads at a block) `leaf_index < blockData.size` but leaf
not written → return 0 with `success = true`
- Update audit headers
## Summary
Hardens barretenberg verifier code paths and API boundaries based on
security review findings.
### Verifier robustness
- Replace `BB_ASSERT` with context-appropriate failures in verifier code
paths — `throw_or_abort` for parsing/structural invariants, return false
for native verification failures, circuit constraints for recursive
verifiers
- Move IPA SRS size check before circuit construction in
`full_verify_recursive` (fail early on misconfiguration)
- Remove redundant witness-value comparison in recursive IPA verifier
(`assert_equal` is the real enforcement)
- Guard unsigned underflow in batched translator verifier's
num_public_inputs derivation
### Input validation
- 2513: Reject non-canonical field encodings in `MsgPack`
deserialization (`value >= modulus` now throws instead of silently
reducing)
- 2501: Reject trailing data in `ChonkProof` `MsgPack` deserialization
(consistent with hardened `PrivateExecutionStepRaw` loaders)
### API state machine fixes
- 2504: `ChonkStart`: clear stale `loaded_circuit_*` state when starting
a new session, preventing silent reuse of circuits from a prior session
- 2509: `ChonkAccumulate`: clear loaded state immediately after move,
preventing use-after-move on retry if a later step throws
### Debug
- 2479: Fix off-by-one in `has_last_app_been_accumulated` flag
(`num_circuits - 4` → `num_circuits - 3`), debug-only
                  
### Tests
                  
- 2513: Regression test for non-canonical `MsgPack` field encoding
rejection (all 6 field types)
- 2493: Regression test for translator `NonNativeFieldRelation`
accumulator alias attack

---------

Co-authored-by: iakovenkos <sergey.s.yakovenko@gmail.com>
@iakovenkos iakovenkos enabled auto-merge April 6, 2026 14:31
@iakovenkos iakovenkos self-requested a review April 6, 2026 14:31
AztecBot added 2 commits April 6, 2026 15:19
…2233)

## Summary

Fixes bb-rs test failure introduced by #22311 (graceful failures in
verifier code paths).

`Fr::from_u64` was storing values in **little-endian**, but the C++
`field::msgpack_unpack` expects **big-endian** (network byte order).
This was masked before because the C++ side silently reduced
non-canonical values via `to_montgomery_form_reduced()`. After #22311
added strict non-canonical field validation (throwing for values >=
modulus), values like `Fr::from_u64(123)` produce `0x7B<<248` which
exceeds the BN254 Fr modulus (`0x30644e72...`) and cause a C++ exception
that propagates through the FFI boundary, aborting the Rust test process
with "Rust cannot catch foreign exceptions".

## Fix

One-line change: store the u64 value in bytes 24..32 as big-endian
instead of bytes 0..8 as little-endian.

## Test plan

- [x] Reproduced failure locally — `test_pedersen_commit_deterministic`
crashes with SIGABRT
- [x] Applied fix — all 70 bb-rs tests pass (3 perf tests ignored)
- [ ] `./bootstrap.sh ci` running
…nical field encoding (#22333)

## Summary

Fixes CI failure in `merge-train/barretenberg` caused by the interaction
of #22233 (Fr::from_u64 big-endian encoding) and #22311 (strict
non-canonical field validation).

The `corruptProofFields()` test helper was flipping high-order bytes
(indices 0,1) of field elements, creating values exceeding the BN254 Fr
modulus. The strict validation now rejects these at deserialization time
with `non-canonical encoding (value >= modulus)` before they reach the
verifier.

**Fix**: Flip low-order bytes (indices 30,31) instead — still corrupts
the proof but keeps field values canonical.

## Test plan
- [x] `batch_verifier.test.ts` — all 4 tests pass locally (previously 1
failing)

ClaudeBox log: https://claudebox.work/s/e5146dc92fea29dd?run=1
@iakovenkos iakovenkos added this pull request to the merge queue Apr 6, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 6, 2026
AztecBot and others added 2 commits April 6, 2026 17:47
## Summary

Fixes WASM CI failure for `PrimeFieldTest/0.MsgpackRejectsNonCanonical`
in `ecc_tests`.

WASM builds define `BB_NO_EXCEPTIONS`, which makes `throw_or_abort()`
call `abort()` instead of throwing `std::runtime_error`. The test uses
`EXPECT_THROW` which cannot catch an abort — the WASM process crashes
with `wasm trap: unreachable`.

Wraps the test with `#ifndef BB_NO_EXCEPTIONS` and adds a `GTEST_SKIP()`
fallback, matching the existing pattern in `c_bind_exception.test.cpp`.

## Test plan
- [x] `ecc_tests --gtest_filter=*MsgpackRejectsNonCanonical*` — all 6
field types pass on native build

ClaudeBox log: https://claudebox.work/s/e5146dc92fea29dd?run=3
@iakovenkos iakovenkos enabled auto-merge April 6, 2026 18:40
@iakovenkos iakovenkos added this pull request to the merge queue Apr 6, 2026
Merged via the queue into next with commit 51c63db Apr 6, 2026
24 checks passed
federicobarbacovi pushed a commit that referenced this pull request Apr 7, 2026
…22347)

## Summary
Fixes nightly barretenberg debug build failure (CI run
https://github.com/AztecProtocol/aztec-packages/actions/runs/24066248159).

**Root cause:**
`HonkRecursionConstraintTestWithoutPredicate/2.GenerateVKFromConstraints`
(root rollup circuit, the heaviest test variant) timed out at 601s
against the 600s default timeout. Exit code 124.

**Fix:**
- Bump timeout from 600s to 900s for `HonkRecursionConstraintTest` and
`ChonkRecursionConstraintTest` suites
- Add `ChonkRecursionConstraintTest` to the CPUS=4:MEM=8g resource group

Several other tests in these suites are also close to the 600s limit
(503s, 519s), so this prevents future timeouts as debug build overhead
grows.

**Note:** PR #22346 (from earlier session) is a no-op — the
logderivative fix it applies already exists on `next` via
merge-train/barretenberg (#22324). The actual failure was this timeout.

ClaudeBox log: https://claudebox.work/s/5f7d3bb56d7d756f?run=1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants